Skip to content

Move EraBasedProtocolParametersUpdate to Cardano.Api.Compatible.ProtocolParametersUpdate - #1336

Merged
Jimbo4350 merged 2 commits into
masterfrom
jordan/move-era-based-pparams-update
Sep 11, 2026
Merged

Move EraBasedProtocolParametersUpdate to Cardano.Api.Compatible.ProtocolParametersUpdate#1336
Jimbo4350 merged 2 commits into
masterfrom
jordan/move-era-based-pparams-update

Conversation

@Jimbo4350

@Jimbo4350 Jimbo4350 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Changelog

- description: |
    Move `EraBasedProtocolParametersUpdate`, its era-sliced record types and the pre-Conway `UpdateProposal` machinery into a new module `Cardano.Api.Compatible.ProtocolParametersUpdate`. `Cardano.Api.ProtocolParameters` re-exports it, so its export list is unchanged.
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  # - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
   - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...
# uncomment at least one main project this PR is associated with
  projects:
   - cardano-api
  # - cardano-api-gen
  # - cardano-rpc
  # - cardano-wasm

Context

Follow-up to #1335 (stacked on it). Moves the era-based update-proposal machinery out of Cardano.Api.ProtocolParameters into Cardano.Api.Compatible.ProtocolParametersUpdate: EraBasedProtocolParametersUpdate, its era-sliced records, createEraBasedProtocolParamUpdate, createPParams, UpdateProposal and its to/fromLedger* conversions. The UpdateProposal cluster must move with the GADT: it contains it, and the GADT's FromCBOR instance depends on fromLedgerPParamsUpdate, which uses the private slice helpers.

Cardano.Api.ProtocolParameters re-exports the new module, so its export list and Cardano.Api are unchanged. Cardano.Api.Compatible re-exports it too.

How to trust this PR

  • Pure code motion plus import/export plumbing; no function bodies changed.
  • The export list of Cardano.Api.ProtocolParameters is identical to master.
  • cabal build cardano-api --enable-tests is clean under -Werror.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

🤖 Generated with Claude Code

{-# LANGUAGE ConstraintKinds #-}
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE DeriveDataTypeable #-}
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE DeriveDataTypeable #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE NamedFieldPuns #-}
@Jimbo4350
Jimbo4350 force-pushed the jordan/move-era-based-pparams-update branch from 302fc0d to 1f1acd6 Compare September 10, 2026 17:58
Base automatically changed from jordan/remove-protocol-parameters to master September 10, 2026 18:37

@palas palas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍. (Just code moving from one module to another.)

One question

Comment on lines +46 to +47
-- * Data family instances
, AsType (..)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-- * Data family instances
, AsType (..)
-- * Data family instances
, AsType (AsUpdateProposal)

Should we qualify this export? Apparently, it will export things like AsPaymentKey, AsWord8, and AsByteStringLazy, if we don't qualify it

Copilot AI lite review requested due to automatic review settings September 11, 2026 15:54
…colParametersUpdate

Relocates the era-based protocol parameter update GADT, its era-sliced
record types, createEraBasedProtocolParamUpdate, createPParams and the
pre-Conway UpdateProposal machinery into a new Compatible module. The
UpdateProposal cluster has to move with the GADT: UpdateProposal contains
it, and its FromCBOR instance depends on fromLedgerPParamsUpdate, which in
turn uses the private pparamsUpdateTo* helpers.

Cardano.Api.ProtocolParameters re-exports the new module, so its export
list and Cardano.Api are unchanged. Cardano.Api.Compatible re-exports the
new module as well.
AsType (..) on a data family re-exports every in-scope instance
constructor, not just the one defined in this module.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved issues were identified, and existing exports are preserved.

Pull request overview

Moves era-based protocol parameter update machinery into a dedicated compatibility module while preserving existing exports.

Changes:

  • Added and re-exported ProtocolParametersUpdate.
  • Registered the new module in Cabal.
  • Added a refactoring changelog entry.
File summaries
File Description
cardano-api/src/Cardano/Api/ProtocolParameters.hs Re-exports the moved definitions.
cardano-api/src/Cardano/Api/Compatible/ProtocolParametersUpdate.hs Contains the moved update machinery.
cardano-api/src/Cardano/Api/Compatible.hs Re-exports the new module.
cardano-api/cardano-api.cabal Exposes the new module.
.changes/move-era-based-protocol-parameters-update.yml Documents the refactoring.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Jimbo4350
Jimbo4350 added this pull request to the merge queue Sep 11, 2026
Merged via the queue into master with commit ab85f88 Sep 11, 2026
31 checks passed
@Jimbo4350
Jimbo4350 deleted the jordan/move-era-based-pparams-update branch September 11, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants